ENH: Plotting both power on and off drag curves in a single plot. #547
Conversation
juliomachad0
added
Enhancement
github-actions
bot
requested review from
giovaniceotto,
Gui-FernandesBR,
MateusStano and
phmbressan
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## develop #547 +/- ##
===========================================
- Coverage 72.32% 72.31% -0.02%
===========================================
Files 56 56
Lines 9399 9416 +17
===========================================
+ Hits 6798 6809 +11
- Misses 2601 2607 +6 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Excellent PR, I went through the PR description and everything was transparent enough.
I'm adding some commentaries with concerns that I have, it would be good to discuss them if needed. Also, please let me know if there's anything you don't understand at a first glance.
Welcome to the Matplotlib School of Engineering!
MNT: removing the unnecessary 'return None' line Co-authored-by: Gui-FernandesBR <[email protected]>
There was a problem hiding this comment.
LGTM,
Thank you for your first contribution.
It does solve the problem and make our lives better when seeing all those plots.
As a suggestion, mext PR I recommend you to also include a picture of the plot you make so we can have a preview before reviewing it.
Pull request type
Checklist
black rocketpy/ tests/) has passed locallypytest --runslow) have passed locallyCHANGELOG.mdhas been updated (if relevant)Current behavior
Before the changes, the drag curves were plotted separated.
New behavior
Now, with the changes made, there is only a plot with both curves.
Breaking change
Additional information
The methods power_on_drag and power_off_drag of the _RocketPlots class were removed.